Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero #302

sebastien-rosset · 2024-07-05T17:50:50Z

Problem statement

When the polar contains a zero value, a parsing warning is created, but no log is generated in opencpn.log. This makes it more difficult to troubleshoot problems.
When the above issue occurs, the first element of the Polar::wind_speed is set to NaN. It seems it should be set to zero.
When the first element of the Polar::wind_speed is set to NaN, the Polar::Speed function may return Nan, and a message is printed to stdout. It should probably have been logged to opencpn.log, which makes it easier to troubleshoot.

Example print statements to stdout when polar contains NaN:

polar failed bad! 18.487259 7.997830 21.487259 nan
polar failed bad! 18.487259 7.997830 24.487259 nan
polar failed bad! 18.487259 7.997830 27.487259 nan
polar failed bad! 18.487259 7.997830 30.487259 nan
polar failed bad! 18.487259 7.997830 33.487259 nan
polar failed bad! 18.487259 7.997830 36.487259 nan
polar failed bad! 18.487259 7.997830 39.487259 nan

Changes in this PR

When the plugin opens a polar and warning-level parse issues are found, log the parse warnings to opencpn.log. This is useful for troubleshooting route calculation issues. For example:

11:32:18.975 MESSAGE Polar.cpp:331 Test-TWS-0-20+60.pol line 2: Warning: 0 values found in polar.
These measurements will be interpolated.
To specify interpolated, leave blank values.
To specify course as 'invalid' use 0.0 rather than 0

When a polar contains the 0 value, initialize the first element of the Polar::wind_speed array with the 0 value instead of NaN.
1. Perhaps setting the first element of the Polar::wind_speed array to NaN was intentional?? See analysis below.
2. When a route is calculated based on polars that contain the zero value, this ends up causing the route calculation to fail.
When route calculation fails, use wxLogMessage instead of printf to report the problem. Improve the message to make it easier to troubleshoot the issue.

With this PR, the print statements are now written to opencpn.log:

10:58:31.752 MESSAGE ocpn_frame.cpp:5590 LOGBOOK:  2024-07-07 17:58:31 UTC  DR Lat   33.35800 Lon  -79.28200
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 21.487259 VB: nan H: 3.000000. Error: 6
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 24.487259 VB: nan H: 6.000000. Error: 6
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 27.487259 VB: nan H: 9.000000. Error: 6
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 30.487259 VB: nan H: 12.000000. Error: 6
10:58:59.467 WARNING RouteMap.cpp:673 polar failed. V: 18.487259 VW: 7.997830 B: 33.487259 VB: nan H: 15.000000. Error: 6

Analysis

When a polar is loaded, some warnings are generated with PARSE_WARNING, but these warnings are not necessarily displayed anywhere. The warnings are not logged in opencpn.log either. This can make route failure harder to understand.

In particular, when a polar file contains a 0 value, Polar::Open adds the NaN value to the first entry in the Polar::wind_speeds array. For example, given the following polar:

twa/tws;6;8;10;12;14;16;20;60
0;0;0;0;0;0;0;0;0
50;5.4;6.5;7.1;7.4;7.5;7.6;7.8;0

The Polar::wind_speeds array is populated as [NaN, 5.4, 5.6, 6, 6.1, 6, 5.6, 5.0, 4.3]
Relevant code:

weather_routing_pi/src/Polar.cpp

Lines 299 to 315 in 97f2a08

    
           double s = NAN; 
        
           if((token = strtok_polar(NULL, &saveptr))) { 
        
               if(!strcmp(token, "0") && !warn_zeros) { 
        
                   PARSE_WARNING(_("Warning: 0 values found in polar.\n" 
        
                                   "These measurements will be interpolated.\n" 
        
                                   "To specify interpolated, leave blank values.\n" 
        
                                   "To specify course as 'invalid' use 0.0 rather than 0\n")); 
        
                   warn_zeros = true; 
        
               } else if(*token) { 
        
                   s = strtod(token, 0); 
        
                   if(s < .05) 
        
                       s = 0; 
        
               } 
        
           } else 
        
               PARSE_WARNING(_("Too few measurements.")); 
        
           wind_speeds[VWi].orig_speeds.push_back(s);

The NaN value in the wind_speeds array ends up causing Polar::Speed() to return VB=NaN at

weather_routing_pi/src/Polar.cpp

Line 537 in 97f2a08

double VB11 = ws1.speeds[W1i], VB12 = ws1.speeds[W2i];

sebastien-rosset · 2024-07-05T19:54:11Z

src/Polar.cpp

@@ -304,6 +304,7 @@ bool Polar::Open(const wxString &filename, wxString &message)
                                        "To specify interpolated, leave blank values.\n"
                                        "To specify course as 'invalid' use 0.0 rather than 0\n"));
                        warn_zeros = true;
+                        s = 0;


Was it intentional that s was set to NaN for the first element?

seandepagnier · 2024-07-07T00:31:27Z

it was intentional... 0 means the boat is not moving. NaN means we do not know the boat speed.. but maybe it is still broken?

sebastien-rosset · 2024-07-07T03:50:45Z

it was intentional... 0 means the boat is not moving. NaN means we do not know the boat speed.. but maybe it is still broken?

Thanks, the history is helpful. I will check again and get back.

sebastien-rosset · 2024-07-07T18:25:38Z

Based on this code:

                  PARSE_WARNING(_("Warning: 0 values found in polar.\n"
                                        "These measurements will be interpolated.\n"
                                        "To specify interpolated, leave blank values.\n"
                                        "To specify course as 'invalid' use 0.0 rather than 0\n"));

and:

** blank = Interpolate from nearby values, if at least one set of
values of tws/twd are entered in the column/row.
*** 0 = Not recognized by the program, will remove these it edited.
Sometimes a polar will contain 0's. They should be removed.
*** 0.0 = Specifies invalid (cannot be used). Boat won't move and can't
go there.

and:

<property name="label">Leave any cell blank to automatically interpolate from nearby values.  
Use a value of 0.0 to specify invalid (cannot be used)&#x0A;
View the polar plot in the boat dialog while editing the polar.</property>

There are four possible values in a polar:

Value	Description
> 0	Normal boat speed value
0.0	Boat speed is considered "invalid"
empty	Value will be interpolated using neighboring cells
0	The first time `"0"` is found in the polar, the cell value is set to `NaN`. This means the value will be interpolated using neighboring cells. Subsequent "0" values in the polar are stored as `0.0` i.e. boat speed is zero.

Values are interpolated only if either:

There are lower and higher wind speed values.
There are lower and higher angle values.

In other cases, the polar value is not interpolated and remains NaN.

Once the polar values have been loaded and the plugin calculates a route, the value NaN is considered invalid. I.e. the plugin will not use this path.

I'm wondering if this logic could be simplified.

Do we really need to treat "0" and "0.0" differently? It's not intuitive.
Why do we need to treat the first "0" value as a special case (converted to NaN) , whereas all other "0" string values are converted to 0.0 numerical values.
Maybe I'm missing something, but what is the rationale for the "0" string value logic? This may be naive, but if I see the value "0", I think that means the boat is not moving for that wind speed and angle; for example, if the boat is directly upwind (TWA is 0) then it makes sense that the boat speed is 0.
1. If the "0" value is in the top-left cell, the value is not interpolated, hence it will be set to NaN. But if the wind angle is zero, it makes sense that the boat speed is 0.0, i.e. the boat speed is not "unknown".
2. If "0" is present in the middle of the polar file, then the 0 value is replaced with the interpolated value based on the neighboring cells. And all other "0" string values will be set to the 0.0 numerical value.

For case 3.i, the code hits this path:
https://github.com/seandepagnier/weather_routing_pi/pull/302/files#diff-e6e0fa7dd681f90e3e13a71ae1102a4bb0c52ae1173eb1dad0865264df6ed903R673
and prints failure messages such as:

polar failed bad! 18.487259 7.997830 21.487259 nan
polar failed bad! 18.487259 7.997830 24.487259 nan
polar failed bad! 18.487259 7.997830 27.487259 nan
polar failed bad! 18.487259 7.997830 30.487259 nan

It the logic remains as is, then perhaps the values in .opencpn/plugins/weather_routing/polars/Example/Test-TWS-0-20+60.pol for the first row should be changed from "0" to "0.0".

sebastien-rosset commented Jul 5, 2024

View reviewed changes

sebastien-rosset marked this pull request as ready for review July 5, 2024 20:03

sebastien-rosset changed the title ~~Log polar error code~~ Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero Jul 5, 2024

sebastien-rosset added 3 commits July 6, 2024 21:10

Log polar error code

3695134

Handle scenario when polar contains NaN value

0a54f91

Handle scenario when polar contains NaN value

2df2559

sebastien-rosset force-pushed the polar_error_code branch from 311a35e to 2df2559 Compare July 7, 2024 04:10

sebastien-rosset marked this pull request as draft July 7, 2024 04:16

sebastien-rosset added 2 commits July 7, 2024 06:49

Revert setting speed to 0 when importing polars

8596f7f

use isnan function

770fd60

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero #302

Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero #302

sebastien-rosset commented Jul 5, 2024 •

edited

Loading

sebastien-rosset Jul 5, 2024

seandepagnier commented Jul 7, 2024

sebastien-rosset commented Jul 7, 2024

sebastien-rosset commented Jul 7, 2024 •

edited

Loading

	double s = NAN;
	if((token = strtok_polar(NULL, &saveptr))) {
	if(!strcmp(token, "0") && !warn_zeros) {
	PARSE_WARNING(_("Warning: 0 values found in polar.\n"
	"These measurements will be interpolated.\n"
	"To specify interpolated, leave blank values.\n"
	"To specify course as 'invalid' use 0.0 rather than 0\n"));
	warn_zeros = true;
	} else if(*token) {
	s = strtod(token, 0);
	if(s < .05)
	s = 0;
	}
	} else
	PARSE_WARNING(_("Too few measurements."));

	wind_speeds[VWi].orig_speeds.push_back(s);

Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero #302

Are you sure you want to change the base?

Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero #302

Conversation

sebastien-rosset commented Jul 5, 2024 • edited Loading

Problem statement

Changes in this PR

Analysis

sebastien-rosset Jul 5, 2024

Choose a reason for hiding this comment

seandepagnier commented Jul 7, 2024

sebastien-rosset commented Jul 7, 2024

sebastien-rosset commented Jul 7, 2024 • edited Loading

sebastien-rosset commented Jul 5, 2024 •

edited

Loading

sebastien-rosset commented Jul 7, 2024 •

edited

Loading